perf(core): count diff lines without materializing display rows - #5243
Open
liuxiaocs7 wants to merge 2 commits into
Open
perf(core): count diff lines without materializing display rows#5243liuxiaocs7 wants to merge 2 commits into
liuxiaocs7 wants to merge 2 commits into
Conversation
Share a structural scanner between diff counting and display parsing. Skip full line splitting, body substrings and display-row allocation when only additions and deletions are requested. Preserve hunk semantics and display row shapes, with boundary and seeded differential coverage. Include a reproducible benchmark that emits raw samples: the 100,000-line fixture counts 2.86x faster, with heapUsed delta reduced from 11.02 MiB to 576 B. Fixes apache#5242 Generated-by: Codex
liuxiaocs7
force-pushed
the
perf/count-diff-without-display-rows
branch
from
September 12, 2026 19:13
0573367 to
dcfe002
Compare
Add a deterministic regression test to the normal core suite that probes the synchronous count call for display-row pushes and full diff splitting. Restore prototype methods in finally before assertions or test-runner work. Verify the test rejects the baseline (four display rows and one split) and passes on the scanner implementation without timing or heap thresholds. Refs apache#5242 Generated-by: Codex
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #5242
countDiffLineStatspreviously built every display row and a complete split-line array just to return additions/deletions. Share one structural scanner withparseUnifiedDiffRows; counting now skips display objects and body substrings while preserving hunk semantics, row shapes, and public APIs.Includes boundary tests, 1,000 seeded multi-file fixtures with independently generated expected rows/counts, and a reproducible benchmark. The 100,000-line replacement fixture counts 2.86× faster and creates zero display rows.
Verification
npm run lint,npm run format:check,npm run build,npm run typecheck.npx --no-install knip --workspace apps/desktopandnpx --no-install knip --workspace packages/ui.finallybefore assertions. This guard runs in CI without timing or heap thresholds. The benchmark retains the same allocation assertions across all seven scenarios; the semantic tests also pass against the baseline to verify compatibility.Benchmark
Apple M4 Pro, macOS arm64, Node v24.14.0 / V8 13.6.233.17-node.41. Baseline:
d2e1be5db93101ffcc0fb6a107a0d51764ace28b. Eight warmup batches, 21 timing samples with alternating version order, forced GC before each batch; medians below are milliseconds per call. Fixture generation, Git/I/O and rendering are outside timing.For 100,000 replacement lines, median per-call
heapUseddelta dropped 11.021 MiB → 576 B, and display objects 100,001 → 0. The 1,000-hunk input dropped 11.339 MiB → 272,408 B because header regex matches still allocate. Heap measurements use nine separate samples after forced GC; these are before/after deltas, not total allocated bytes or exact peaks. The 576 B result is near the measurement overhead.Display parsing was also checked for identical output and no measured slowdown: the 100,000-line fixture changed 2.6189 → 1.9725 ms. Long-line inputs show little speedup because text scanning dominates. The 500,000-line fixture is a core stress case exceeding the Git review input cap; these numbers do not imply equivalent end-to-end UI gains.
Reproduce:
Benchmark script.
AI use
Select exactly one:
Tool(s) and scope: Codex assisted with diagnosis, implementation, tests, benchmarking, and this submission. This automated submission was requested by @liuxiaocs7, the human contributor of record. Both commits include
Generated-by: Codex.Checklist
Does this PR entail a change in behavior?